Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(checker): add ability to check multiple files at once #127

Merged
merged 1 commit into from
Apr 15, 2018

Conversation

ikatyang
Copy link
Member

@ikatyang ikatyang commented Apr 10, 2018

Fixes #126

before

tslint-config-prettier-check ./tslint.yml && tslint-config-prettier-check ./test/tslint.yml

after

tslint-config-prettier-check ./tslint.yml ./test/tslint.yml

@ikatyang ikatyang requested a review from alexjoverm April 10, 2018 13:54
@astorije
Copy link
Contributor

Why oh why didn't I look at open PRs prior to opening #129...

Thanks for this @ikatyang, looks much better than my quick-and-dirty version!

@astorije
Copy link
Contributor

astorije commented Apr 13, 2018

Actually, does this support glob-ing too? Like tslint-config-prettier-check ./**/tslint.yml?

@ikatyang
Copy link
Member Author

It's based on your shell:

$ node -e "console.log(process.argv)" ./fixtures/tslint.*.json
[ '/usr/local/Cellar/node/9.10.1/bin/node',
  './fixtures/tslint.all.json',
  './fixtures/tslint.empty.json',
  './fixtures/tslint.error.json',
  './fixtures/tslint.false.json' ]

And also you can try it with this PR:

git clone https://github.com/alexjoverm/tslint-config-prettier --branch feat/check-multiplie-config-files
cd tslint-config-prettier
npm install
node bin/check.js ./fixtures/tslint.*.json

@ikatyang
Copy link
Member Author

And the reason why I don't want to use some node-glob-like dependency is that this package is mainly just a config file, that kind of dependency is somehow too heavy for us, a simple tool should be enough.

glob@7.1.2
├── fs.realpath@1.0.0
├─┬ inflight@1.0.6
│ ├── once@1.4.0 deduped
│ └── wrappy@1.0.2
├── inherits@2.0.3
├─┬ minimatch@3.0.4
│ └─┬ brace-expansion@1.1.11
│   ├── balanced-match@1.0.0
│   └── concat-map@0.0.1
├─┬ once@1.4.0
│ └── wrappy@1.0.2 deduped
└── path-is-absolute@1.0.1

@astorije
Copy link
Contributor

Yeah my question was stupid, of course it support globbing if you can pass it multiple files, duh.

And the reason why I don't want to use some node-glob-like dependency is that this package is mainly just a config file, that kind of dependency is somehow too heavy for us, a simple tool should be enough.

I totally understand that, I had the exact same reasoning when opening #129.

Props to you for taking this up! Can't wait to simplify my yarn script :)

@ikatyang ikatyang merged commit e3e8012 into master Apr 15, 2018
@ikatyang ikatyang deleted the feat/check-multiplie-config-files branch April 15, 2018 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants